feat(asyncpg): Add query source to execute, executemany, and cursor#6242
feat(asyncpg): Add query source to execute, executemany, and cursor#6242ericapisani wants to merge 9 commits into
Conversation
Migrates the asyncpg integration to use `record_sql_queries_supporting_streaming` and `StreamedSpan`, adding support for the `stream` trace lifecycle. Tests are parameterized to cover both static and streaming modes. Fixes PY-2305 Fixes #6003
…an has been created. This allows the traces_sampler to see the attributes and make decisions based on them
Add add_query_source() calls to execute(), executemany(), and cursor() methods. Previously only fetch() had query source tracking. Handle StreamedSpan and regular span cases separately since StreamedSpan requires the call while still inside the context manager. Add comprehensive test coverage for query source tracking across all wrapped methods.
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 10.66s All tests are passing successfully. ❌ Patch coverage is 0.00%. Project has 15167 uncovered lines. Files with missing lines (1)
Generated by Codecov Action |
…to-methods-that-were-missing-it
sentrivana
left a comment
sentrivana
left a comment
There was a problem hiding this comment.
One comment re: naming in tests https://github.com/getsentry/sentry-python/pull/6242/changes#r3225427754
Otherwise lgtm!
| conn: Connection = await connect(PG_CONNECTION_URI) | ||
| if span_streaming: | ||
| items = capture_items("span") | ||
| with sentry_sdk.traces.start_span(name="test_transaction"): |
There was a problem hiding this comment.
Can we call this test_span or test_segment in the span first code path? Since there are no transactions in span first
Also if we mention transactions in other span first tests we should change that 🙏🏻
There was a problem hiding this comment.
Why do we add query source to _wrap_cursor_creation()?
My understanding is that the add_query_source() function is intended to add the query source to functions that execute SQL queries, which are only the following:
asyncpg.Connection.executeasyncpg.Connection._executeasyncpg.Connection._executemany
The parameter description says the following:
sentry-python/sentry_sdk/consts.py
Line 1559 in 17cc8c7
Can we ensure that only spans for these functions have query source added?
When this PR was initially created, I hadn't yet noticed the underlying issue with how we've patched cursors, and that this only captured the creation of the At this point, due to the introduction of #6252 , this PR is somewhat out of date in that cursor creation is no longer patched - the underlying This means that, with those changes:
and we'll want to add query sources to those. |
3 participants
Summary
add_query_source()calls to_wrap_connection_methodand_wrap_cursor_creation, which coverexecute(),executemany(),prepare(), andcursor(). Previously onlyfetch()had query source tracking via the patched_wrap_executemethod.StreamedSpanvs regular span cases separately (StreamedSpan needs the call inside the context manager).